Skip to content

feat: add skills command to read embedded skill content#1277

Open
dc-bytedance wants to merge 2 commits into
larksuite:mainfrom
dc-bytedance:feat/skill-content-read
Open

feat: add skills command to read embedded skill content#1277
dc-bytedance wants to merge 2 commits into
larksuite:mainfrom
dc-bytedance:feat/skill-content-read

Conversation

@dc-bytedance
Copy link
Copy Markdown
Collaborator

@dc-bytedance dc-bytedance commented Jun 5, 2026

Summary

lark-cli ships one skill per business domain, but skills install through a separate channel and carry their own version, so after a CLI upgrade an on-disk skill can fall out of sync with the binary (the docs v1/v2 drift is a concrete case). This PR adds a lark-cli skills command group that reads skill content compiled into the binary at build time via go:embed, giving AI agents a version-consistent source — and a fallback for when skills are not installed or are stale. It does not replace or thin the on-disk skills.

Changes

  • Add repo-root skills_embed.go (package main): //go:embed skills plus an init() that fs.Sub-roots the tree and injects it into cmd/skill via a package variable, avoiding any change to cmd.Execute's signature.
  • Add internal/skillcontent/reader.go: pure logic over an injected fs.FSList (skills with name/description/frontmatter metadata), ListPath (one directory layer, ls-style, no recursion), ReadSkill, ReadReference, and a shared cleanSubPath traversal guard.
  • Add cmd/skill/skill.go: the skills command group with list (catalog, or one-layer listing of <name> / <name>/<sub>) and read (<name>, <name> <path>, or <name>/<path>); reading the main SKILL.md appends a one-line guidance tip (a separate guidance field under --json).
  • Register the group in cmd/build.go.
  • Add unit tests for both packages (cmd/skill/skill_test.go, internal/skillcontent/reader_test.go).

Test Plan

  • make unit-test passed
  • validate passed (build / vet / unit / integration)
  • local-eval passed (E2E 13/13, skillave N/A — no shortcut/skill content change)
  • acceptance-reviewer passed (5 scenarios + ~10 exploratory sub-scenarios)
  • code security review passed (0 findings)
  • manual verification: lark-cli skills list, skills list lark-doc, skills list lark-doc/references, skills read lark-calendar [--json], skills read lark-doc references/lark-doc-fetch.md, skills read lark-doc/references/lark-doc-fetch.md; error paths (read no-such-skill, read lark-calendar ../../etc/passwd, read, list a b) all exit 2 with empty stdout

Related Issues

N/A

Summary by CodeRabbit

  • New Features

    • Added a top-level "skills" CLI group with skills list and skills read (supports raw and --json modes).
    • CLI can accept optional embedded skill content at startup; when absent the feature is disabled with a single warning.
    • Reading a skill's main page includes a separate one-line guidance tip while referenced files return raw content.
  • Quality

    • Extensive tests for CLI behavior, listing, reading, frontmatter parsing, and error cases.
    • Stronger input validation and path-traversal protections.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 5, 2026

Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place.

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an embedded skills filesystem, a skillcontent.Reader, Cobra skills command with list and read (raw/JSON) including guidance for SKILL.md, build-time embedding and wiring into the CLI factory/build pipeline, and tests for reader and CLI behavior.

Changes

Skills command

Layer / File(s) Summary
Skill content reader abstraction
internal/skillcontent/reader.go, internal/skillcontent/reader_test.go
Reader backed by injected fs.FS enumerates top-level skills and parses SKILL.md frontmatter, lists one-level directory children, reads SKILL.md and referenced files, normalizes/validates relative paths to prevent traversal, and returns typed validation/internal errors; covered by unit tests.
CLI command implementation
cmd/skill/skill.go, cmd/skill/skill_test.go
Adds NewCmdSkill, skills list [name[/path]] and skills read <name>[/<path>] [path], argument parsing helpers, JSON/raw output modes, guidance injection for SKILL.md, a newReader helper that errors when no embedded FS is provided, and comprehensive CLI tests including traversal and nil-ContentFS cases.
Embedding and registration
skills_embed.go, cmd/build.go, cmd/root.go, internal/cmdutil/factory.go, main.go
Embeds the skills/ directory at build time, exposes it via fs.Sub, provides WithSkillContent(fsys fs.FS) build option, wires SkillContent into Factory, updates Execute to accept BuildOptions, and registers the skills subcommand on the root command.

Sequence Diagram

sequenceDiagram
  participant User
  participant SkillCmd as skills command
  participant Reader as skillcontent.Reader
  participant FS as embedded fs.FS

  User->>SkillCmd: list [skill[/path]]
  SkillCmd->>SkillCmd: validate args (≤1)
  SkillCmd->>Reader: List() or ListPath()
  Reader->>FS: readdir, open SKILL.md
  FS-->>Reader: content, entries
  Reader->>Reader: parse YAML frontmatter
  Reader-->>SkillCmd: []SkillInfo or []DirEntry
  SkillCmd->>SkillCmd: emit JSON envelope
  SkillCmd-->>User: JSON or error

  User->>SkillCmd: read name[/path] [path]
  SkillCmd->>SkillCmd: parseReadTarget()
  SkillCmd->>Reader: ReadSkill() or ReadReference()
  Reader->>FS: stat, open file
  FS-->>Reader: bytes
  Reader-->>SkillCmd: content, cleanPath
  SkillCmd->>SkillCmd: append guidance (SKILL.md only)
  SkillCmd->>SkillCmd: JSON or raw markdown
  SkillCmd-->>User: output or error
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • larksuite/cli#371: Both PRs modify cmd/build.go’s build option wiring; this PR adds WithSkillContent and uses the same build/option infrastructure.

Suggested reviewers

  • zkh-bytedance

"A rabbit finds skills in files embedded tight,
Hops through SKILL.md by day and night,
Lists them clean with JSON light,
Reads references without a fright,
Hooray — the skills command leaps into sight!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.59% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add skills command to read embedded skill content' directly summarizes the main change: introducing a new CLI command group for reading embedded skill content.
Description check ✅ Passed The PR description provides a clear summary, detailed changes list, comprehensive test plan with checkmarks, and explicit 'N/A' for related issues, matching the template structure well.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the size/L Large or sensitive change across domains or core paths label Jun 5, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 5, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@51a72d000657818e82a61d20915cc9f17c3bc2f9

🧩 Skill update

npx skills add dc-bytedance/cli#feat/skill-content-read -y -g

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 89.33333% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.99%. Comparing base (bd07859) to head (51a72d0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/skillcontent/reader.go 87.71% 8 Missing and 6 partials ⚠️
cmd/skill/skill.go 94.11% 3 Missing and 3 partials ⚠️
skills_embed.go 50.00% 2 Missing and 1 partial ⚠️
cmd/build.go 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1277      +/-   ##
==========================================
+ Coverage   70.93%   70.99%   +0.06%     
==========================================
  Files         684      687       +3     
  Lines       65653    65878     +225     
==========================================
+ Hits        46572    46773     +201     
- Misses      15403    15417      +14     
- Partials     3678     3688      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/skill/skill_test.go`:
- Around line 25-33: The run test helper does not isolate CLI config state, so
before calling cmdutil.TestFactory(t, ...) set the environment variable
LARKSUITE_CLI_CONFIG_DIR to a temp dir using
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()); update the run function
(around invocation of cmdutil.TestFactory and creation of NewCmdSkill) to call
t.Setenv(...) first to ensure each test gets an isolated config directory.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 699b2379-1c2e-460f-aae9-d334c1633dd0

📥 Commits

Reviewing files that changed from the base of the PR and between a4a4bd6 and af1d44f.

📒 Files selected for processing (6)
  • cmd/build.go
  • cmd/skill/skill.go
  • cmd/skill/skill_test.go
  • internal/skillcontent/reader.go
  • internal/skillcontent/reader_test.go
  • skills_embed.go

Comment thread cmd/skill/skill_test.go Outdated
@SunPeiYang996 SunPeiYang996 force-pushed the feat/skill-content-read branch from 8d67578 to b0ac518 Compare June 5, 2026 06:42
@dc-bytedance dc-bytedance force-pushed the feat/skill-content-read branch from b0ac518 to bd4433f Compare June 6, 2026 03:56
dc-bytedance added a commit to dc-bytedance/cli that referenced this pull request Jun 6, 2026
Apply the PR larksuite#1277 review:
- Inject the embedded skill tree via a BuildOption + Factory field
  instead of a package global set in init(): skills_embed.go exposes
  skillContentOptions() (degrades with a stderr warning, no panic),
  Execute takes variadic BuildOptions, WithSkillContent wires
  Factory.SkillContent, and cmd/skill reads it from the Factory. Removes
  the package global, init(), panic, and the t.Parallel ban in tests.
- Surface the frontmatter version field in `skills list` for drift
  detection.
- Scope the read guidance tip to the skill's own files and stop
  disparaging direct reads; emit it on stderr in raw mode (stdout stays
  byte-identical to SKILL.md) and in the --json guidance field.
- Use typed structs for the list and read JSON envelopes.
- Share one path splitter (skillcontent.SplitArg) between read and list.
- Skip top-level directories without a SKILL.md in List().
- Add usage examples to `skills list` and `skills read`.
Add a top-level `lark-cli skills` meta command group that reads skill
content compiled into the binary at build time via go:embed, so the
content an AI agent reads always matches the running CLI version. It is
an additional, version-consistent source and a fallback for when skills
are not installed or are stale; it does not replace or thin the on-disk
skills.

Commands:
- `skills list` lists every embedded skill with name, description, and
  the frontmatter metadata block.
- `skills list <name>` / `skills list <name>/<sub>` list one directory
  layer (ls-style, no recursion); entries are skill-prefixed paths that
  can be fed straight back to `read`.
- `skills read <name>` prints the skill's SKILL.md and appends a one-line
  tip steering the model to fetch reference files via this command.
- `skills read <name> <path>` / `skills read <name>/<path>` print any
  file under the skill directory (no tip).
- `--json` wraps output in an envelope; on the main SKILL.md it carries a
  separate `guidance` field instead of merging the tip into content.

Implementation:
- skills/ is embedded from the repo-root package main (go:embed cannot
  cross parent dirs) and injected into cmd/skill via a package variable,
  avoiding any change to cmd.Execute's signature.
- internal/skillcontent is pure logic over an injected fs.FS; reference
  reads and directory listings share one path-traversal guard that
  rejects absolute paths, "..", and "..\\" escapes.
- Errors are typed (validation -> exit 2, file_io -> exit 5); rejected
  paths never write anything to stdout.

Risk is set per leaf subcommand (read); the group carries none, matching
the config/service command groups. The commands need no auth (local
embed reads only).
dc-bytedance added a commit to dc-bytedance/cli that referenced this pull request Jun 6, 2026
Apply the PR larksuite#1277 review:
- Inject the embedded skill tree via a BuildOption + Factory field
  instead of a package global set in init(): skills_embed.go exposes
  skillContentOptions() (degrades with a stderr warning, no panic),
  Execute takes variadic BuildOptions, WithSkillContent wires
  Factory.SkillContent, and cmd/skill reads it from the Factory. Removes
  the package global, init(), panic, and the t.Parallel ban in tests.
- Surface the frontmatter version field in `skills list` for drift
  detection.
- Scope the read guidance tip to the skill's own files and stop
  disparaging direct reads; emit it on stderr in raw mode (stdout stays
  byte-identical to SKILL.md) and in the --json guidance field.
- Use typed structs for the list and read JSON envelopes.
- Share one path splitter (skillcontent.SplitArg) between read and list.
- Skip top-level directories without a SKILL.md in List().
- Add usage examples to `skills list` and `skills read`.
@dc-bytedance dc-bytedance force-pushed the feat/skill-content-read branch from 14faa30 to c6274f5 Compare June 6, 2026 07:39
Squashes the review-response iteration on top of the initial skills
command.

- Inject the embedded tree via an init() calling
  cmd.SetEmbeddedSkillContent (no cmd/skill package global, no panic;
  tests inject through the Factory). main.go stays self-contained so the
  single-file preview build (go build ./main.go) still compiles.
- Surface the frontmatter version field in `skills list`.
- Use typed structs for the list and read JSON envelopes.
- Scope the read guidance tip to the skill's own files, route cross-skill
  "../lark-foo/..." references back through `skills read`, and emit it on
  stderr in raw mode (stdout stays byte-identical to SKILL.md) and in a
  guidance field under --json.
- Share one path splitter (skillcontent.SplitArg) between read and list.
- Skip top-level directories without a SKILL.md in List().
- Add usage examples to `skills list` and `skills read`.
- Isolate config state in the command test helper.
- Slim the embed to agent-readable content (SKILL.md + references/, plus
  lark-whiteboard's routes/ and scenes/), excluding machine-resource
  assets/ and scripts/: release binary 26.7 -> 23.4 MB, no change to what
  the commands serve.
@dc-bytedance dc-bytedance force-pushed the feat/skill-content-read branch from 962e1ab to 51a72d0 Compare June 6, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant